Skip to content

feat(dashboard): add data lifecycle feedback and purge capability#812

Open
yasinBursali wants to merge 4 commits intoLight-Heart-Labs:mainfrom
yasinBursali:feat/data-lifecycle-dashboard
Open

feat(dashboard): add data lifecycle feedback and purge capability#812
yasinBursali wants to merge 4 commits intoLight-Heart-Labs:mainfrom
yasinBursali:feat/data-lifecycle-dashboard

Conversation

@yasinBursali
Copy link
Copy Markdown
Contributor

What

  • Add data_info to disable/uninstall API responses showing preserved data size and purge command
  • Add DELETE /api/extensions/{id}/data purge endpoint with confirm gate
  • Add GET /api/storage/orphaned for detecting abandoned data directories
  • Add Purge Data button to dashboard with data-aware confirmation dialogs

Why

  • Users get no feedback about preserved data when disabling extensions
  • No way to reclaim disk space through the dashboard
  • Orphaned data from removed extensions wastes disk invisibly
  • Mirrors the CLI dream purge command (PR 2) in the web UI

How

  • data_info: _get_service_data_info() scans data/{service_id} via dir_size_gb(), returns size/path/purge_command or null
  • Purge endpoint: Validates service_id (regex), blocks core services (403), requires disabled state (400), path traversal prevention (resolve() + is_relative_to()), requires {"confirm": true} in body, measures size before shutil.rmtree, verifies removal
  • Orphaned scan: Iterates data/ directories, excludes SERVICES keys + system dirs (models, config, user-extensions, extensions-library)
  • UI: Amber "Purge Data" button for disabled user extensions, red destructive confirmation dialog, data size display after disable/uninstall

Three Pillars Impact

  • Install Reliability: No installer changes
  • Broad Compatibility: Standard Python pathlib + shutil, all platforms
  • Extension Coherence: Strengthens data lifecycle — disable (preserve) → purge (delete) is now explicit in both CLI and dashboard

Modified Files

  • dashboard-api/routers/extensions.py_get_service_data_info(), data_info in responses, purge + orphaned endpoints
  • dashboard/src/pages/Extensions.jsx — purge button, confirmation dialog, data_info display

Testing

Automated

  • Python compile: PASS
  • Dashboard build: PASS (977ms)
  • Existing tests: 173 pass

Manual

  • Disable extension → response includes data_info with size
  • DELETE /api/extensions/n8n/data without confirm: true → 400
  • DELETE /api/extensions/dashboard/data → 403 (core service)
  • DELETE /api/extensions/n8n/data (enabled) → 400
  • DELETE /api/extensions/n8n/data with {"confirm": true} → success + size freed
  • GET /api/storage/orphaned → lists dirs not in SERVICES
  • UI: Purge button only visible for disabled user extensions
  • UI: Confirmation dialog shows data size

Review

  • CG: ⚠️ APPROVED WITH WARNINGS (dir scan latency on large dirs — non-blocking)
  • Security: PASS (regex + resolve/is_relative_to, confirm gate, core service block)

Platform Impact

  • All platforms supported

Sequence

PR 7 of 7 (Phase 3). Depends on PR 3 (#804 — dir_size_gb extraction)

Copy link
Copy Markdown
Collaborator

@Lightheartdevs Lightheartdevs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Audit: APPROVE

Path traversal prevention is solid: resolve() + is_relative_to(). Service ID validated via _SERVICE_ID_RE. Core service guard (403). Confirmation gate (confirm: true required). The _get_service_data_info() helper is clean and reusable.

Minor notes (non-blocking):

  • shutil.rmtree(data_path, ignore_errors=True) silently swallows errors — the follow-up existence check catches incomplete removal, but root cause (locked files) won't be logged
  • dir_size_gb() is synchronous in an async endpoint — wrap in asyncio.to_thread to avoid blocking the event loop on large data directories
  • Purge button may appear for services with no data directory (user gets 404) — consider hiding when data_info is null
  • orphaned_storage endpoint also scans synchronously — same blocking concern

Depends on #804. Should merge after it.

Copy link
Copy Markdown
Collaborator

@Lightheartdevs Lightheartdevs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revised Audit: REQUEST CHANGES — race condition + missing path check + blocking I/O

Withdrawing my earlier approval after deeper review.

Bug 1 (MEDIUM — Security): Purge does not acquire _extensions_lock()
Race condition with enable_extension: (1) purge checks compose.yaml is gone → proceeds, (2) concurrent enable renames compose.yaml back + starts container, (3) purge shutil.rmtrees data directory under the running container. The purge endpoint should acquire _extensions_lock() and re-check enabled state inside the lock.

Bug 2 (LOW-MEDIUM — Security): _get_service_data_info() has no path traversal protection
The purge endpoint correctly uses resolve() + is_relative_to(), but _get_service_data_info() (called from disable/uninstall responses) constructs the same path WITHOUT those checks. A symlink planted at data/{service_id} would cause dir_size_gb to recursively walk the symlink target, leaking its size.

Fix: Add resolve() + is_relative_to() check in _get_service_data_info() before calling dir_size_gb().

Bug 3 (MEDIUM — Performance): Async endpoints with synchronous blocking I/O
Both purge_extension_data and orphaned_storage are async def but call dir_size_gb() and shutil.rmtree() synchronously. These block the event loop — for large model directories (tens of GB), the entire API hangs for seconds.

Fix: Either change to def (FastAPI auto-runs in threadpool) or wrap blocking calls in asyncio.to_thread().

Bug 4 (LOW — Info disclosure): 500 error leaks absolute path

raise HTTPException(status_code=500, detail=f"Could not fully remove {data_path}...")

data_path is the resolved absolute path. Replace with data/{service_id}.

BLOCKER: Depends on PR #804 for from helpers import dir_size_gb.

What's confirmed solid: Purge path traversal check (resolve + is_relative_to), _SERVICE_ID_RE regex, server-side confirm gate, shutil.rmtree is safe against symlink-swap TOCTOU (resolved path is used), core service guard is dynamic from manifests, frontend purge button correctly scoped to disabled user extensions.

@yasinBursali
Copy link
Copy Markdown
Contributor Author

Addressing review feedback

All 4 bugs fixed:

Bug 1 (Race condition — purge without lock) — Fixed:
purge_extension_data now wraps the entire critical section (enabled-check → path validation → rmtree → post-delete check) inside with _extensions_lock():. Serializes correctly with enable_extension, disable_extension, and uninstall_extension.

Bug 2 (Path traversal in _get_service_data_info) — Fixed:
Added .resolve() + .is_relative_to(Path(DATA_DIR).resolve()) check. Returns None for traversal attempts. Defense-in-depth alongside _SERVICE_ID_RE regex validation.

Bug 3 (Blocking I/O in async endpoints) — Fixed:
Changed both purge_extension_data and orphaned_storage from async def to def. FastAPI automatically runs sync route functions in a threadpool, preventing event loop starvation during shutil.rmtree and dir_size_gb calls.

Bug 4 (Absolute path in 500 error) — Fixed:
Replaced f"Could not fully remove {data_path}..." with f"Could not fully remove data/{service_id}...". No absolute path disclosure.

Additionally: Added ("DELETE", "/api/extensions/{}/data") to test_path_traversal_all_mutations to close a security test gap.

@Lightheartdevs
Copy link
Copy Markdown
Collaborator

Note: The Rust dashboard-api rewrite (#821) merged and deleted the Python files this PR modifies.

The Python router changes in this PR are dead (routers/extensions.py was deleted). The data lifecycle / purge features need to be reimplemented in the Rust codebase. The frontend changes may still be valid if rebased.

Please rebase or rewrite against the current main branch.

@yasinBursali yasinBursali force-pushed the feat/data-lifecycle-dashboard branch from 89d173f to d53ccd2 Compare April 6, 2026 13:44
@yasinBursali
Copy link
Copy Markdown
Contributor Author

All 4 issues from the revised audit are addressed in the current code:

  1. Race condition (lock)purge_extension_data acquires _extensions_lock() and re-checks enabled state inside the lock before proceeding.

  2. Path traversal in _get_service_data_info() — Uses resolve() + is_relative_to() check, returns None if path escapes DATA_DIR.

  3. Blocking I/O — Both purge_extension_data and orphaned_storage are def (not async def), so FastAPI auto-runs them in a threadpool.

  4. Info disclosure — Error message uses data/{service_id} instead of the resolved absolute path.

Also fixed CI failures after rebase:

Rebased on latest main, all checks green.

yasinBursali and others added 4 commits April 6, 2026 20:33
Add data_info to disable/uninstall API responses showing preserved data
size. Add DELETE /api/extensions/{id}/data purge endpoint with confirm
gate, path traversal prevention, and core service protection. Add GET
/api/storage/orphaned for detecting abandoned data directories. Update
Extensions UI with purge button and data-aware confirmation dialogs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…path leak

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove duplicate dir_size_gb (now in helpers.py from upstream)
- Use dir_size_gb from helpers import in main.py instead of local def
- Send JSON body in path traversal test for purge endpoint to avoid
  FastAPI 422 from missing PurgeRequest body

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@yasinBursali yasinBursali force-pushed the feat/data-lifecycle-dashboard branch from d53ccd2 to 41e8200 Compare April 6, 2026 17:34
@yasinBursali
Copy link
Copy Markdown
Contributor Author

Rebased on latest main (post-#829 liquid metal merge).

Merge conflict resolved — 2 conflicts in Extensions.jsx (confirmation dialog):

  • Dialog container: bg-zinc-900 border-zinc-700bg-theme-card border-theme-border
  • Dialog title: text-whitetext-theme-text
  • Confirm button non-destructive style: bg-indigo-500/20 text-indigo-300bg-theme-accent/20 text-theme-accent-light
  • Purge button: bg-zinc-800bg-theme-card
  • Purge action added to both destructive-action check ('uninstall' || 'purge') and confirm button label

All 4 review findings from the revised audit remain addressed (race condition lock, path traversal in _get_service_data_info, sync def endpoints, sanitized error path). All CI checks passing.

Note: the earlier comment about the Rust rewrite (#821) is stale — that PR was reverted (c3e54d2). The Python dashboard-api files are intact on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants